core::num::f16b Rust's 16bit Brain Float - #160859
Conversation
|
Some changes occurred in compiler/rustc_attr_ir cc @jdonszelmann, @JonathanBrouwer This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410
|
|
r? @jieyouxu rustbot has assigned @jieyouxu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
@rustbot reroll |
| } | ||
|
|
||
| fn type_f16b(&self) -> Type<'gcc> { | ||
| bug!("f16b is not supported by the GCC codegen backend") |
There was a problem hiding this comment.
I believe GCC actually supports this type: https://github.com/rust-lang/gccjit.rs/blob/master/src/context.rs#L1482
There was a problem hiding this comment.
Thanks 😄, I will aim to add it in a follow up PR 👍
bf10f8a to
01c5c1b
Compare
|
cc @bjorn3 |
There was a problem hiding this comment.
What is the calling convention of other targets?
There was a problem hiding this comment.
I believe only these files were changed because they have an exhaustive match on Float.
However, my version of abi-cafe found two interesting failures: GCC and Clang are inconsistent on aarch64 and armv7
// callee, compiled with GCC 12
#include <inttypes.h>
#include <string.h>
#include <stdio.h>
#include <stdbool.h>
typedef struct Many1 {
__bf16 f0;
} Many1;
void struct_in_1(Many1 arg0) {
printf("%d", arg0.f0);
}// caller, compiled with clang 23
#include <inttypes.h>
#include <string.h>
#include <stdio.h>
#include <stdbool.h>
typedef struct Many1 {
__bf16 f0;
} Many1;
void struct_in_1(Many1 arg0);
void do_test(void) {
{
Many1 arg0 = { .f0 = (((union { uint16_t bits; __bf16 value; }){ .bits = 49600 }).value) };
printf("%d", arg0.f0);
struct_in_1(arg0);
}
}hits
func struct_in_1's values differed
values (native-endian hex bytes):
expect: C0 C1
caller: C0 C1
callee: 04 00
the value was arg0.f0: rustarithmeticty(f16b)
whose arg was arg0: Many1
The current Rust implementation matches clang, and is hence incompatible with GCC
armv7 with hardware floats also runs into incompatibilities
[target.armv7-unknown-linux-gnueabihf."f16::conv_c::repr_c::clang-nightly_calls_distro-gcc"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16::conv_c::repr_c::distro-gcc_calls_clang-nightly"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16b::conv_c::repr_c::clang-nightly_calls_distro-gcc"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16b::conv_c::repr_c::distro-gcc_calls_clang-nightly"]
busted = "check"
Finally, you can let this ICE on many targets, e.g. mips, powerpc, s390x, sparc
There was a problem hiding this comment.
You can also (e.g. on loongarch64 https://godbolt.org/z/Y3hdhG4e6) emit a __truncsfbf2 libcall that is not provided (probably needs to be added to compiler-builtins).
I think the ICEs are probably a blocker? That needs a mechanism similar to has_reliable_f128.
There was a problem hiding this comment.
Yes I added it because of the exhaustive match statement in mips64.rs and sparc64.rs I've removed it; 40d3f6e and put in a panic!(...).
With regard to has_reliable_f128, are you envisaging a has_reliable_f16b entry on TargetConfig?
There was a problem hiding this comment.
With regard to
has_reliable_f128, are you envisaging ahas_reliable_f16bentry onTargetConfig?
Exactly
There was a problem hiding this comment.
What's your take on those ABI mismatches? We should track that somewhere.
There was a problem hiding this comment.
I'm not particularly certain what mips64 should do.
It doesn't, at least to my knowledge, have hardware support. Given we aren't implementing scalar arithmetic and an f16b can only be created through a bit pattern or vendor intrinsics, I can't immediately see a practical application? Hence a panic! seems like a pragmatic choice for the time being.
There was a problem hiding this comment.
No comment on whether this is the right approach, but always use span_bug! or at least bug! where possible in the compiler, rather than panic. It gives much more useful info.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
I believe only these files were changed because they have an exhaustive match on Float.
However, my version of abi-cafe found two interesting failures: GCC and Clang are inconsistent on aarch64 and armv7
// callee, compiled with GCC 12
#include <inttypes.h>
#include <string.h>
#include <stdio.h>
#include <stdbool.h>
typedef struct Many1 {
__bf16 f0;
} Many1;
void struct_in_1(Many1 arg0) {
printf("%d", arg0.f0);
}// caller, compiled with clang 23
#include <inttypes.h>
#include <string.h>
#include <stdio.h>
#include <stdbool.h>
typedef struct Many1 {
__bf16 f0;
} Many1;
void struct_in_1(Many1 arg0);
void do_test(void) {
{
Many1 arg0 = { .f0 = (((union { uint16_t bits; __bf16 value; }){ .bits = 49600 }).value) };
printf("%d", arg0.f0);
struct_in_1(arg0);
}
}hits
func struct_in_1's values differed
values (native-endian hex bytes):
expect: C0 C1
caller: C0 C1
callee: 04 00
the value was arg0.f0: rustarithmeticty(f16b)
whose arg was arg0: Many1
The current Rust implementation matches clang, and is hence incompatible with GCC
armv7 with hardware floats also runs into incompatibilities
[target.armv7-unknown-linux-gnueabihf."f16::conv_c::repr_c::clang-nightly_calls_distro-gcc"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16::conv_c::repr_c::distro-gcc_calls_clang-nightly"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16b::conv_c::repr_c::clang-nightly_calls_distro-gcc"]
busted = "check"
[target.armv7-unknown-linux-gnueabihf."f16b::conv_c::repr_c::distro-gcc_calls_clang-nightly"]
busted = "check"
Finally, you can let this ICE on many targets, e.g. mips, powerpc, s390x, sparc
| }, | ||
| Primitive::Float(float) => match float { | ||
| Float::F16 | Float::F32 => "f32", | ||
| Float::F16 | Float::F16B | Float::F32 => "f32", |
There was a problem hiding this comment.
is that right? LLVM just crashes on bf16 right now, so it's probably at least untested?
There was a problem hiding this comment.
I could be mistaken, however I don't think WASM supports bf16? I've made it panic!(...) for now; 40d3f6e
53d3660 to
40d3f6e
Compare
This comment has been minimized.
This comment has been minimized.
da620d0 to
0ed984f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred in cfg and check-cfg configuration cc @Urgau
cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
|
I'm not a good reviewer for this change. Can somebody here pick it up rather than blindly rerolling? |
|
r? me |
| #[cfg_attr(feature = "nightly", derive(StableHash))] | ||
| pub enum Float { | ||
| F16, | ||
| F16B, |
There was a problem hiding this comment.
| F16B, | |
| /// `f16b`. This is not a builtin type in Rust (it is exposed as a lang item), | |
| /// but it is a builtin type in LLVM so needs to be explicitly represented | |
| /// in the backend. | |
| F16B, |
… and backend handling. GCC and Cranelift explicitly unsupported
…long with traits agreed on in the RFC
…6b` to be treated as a primitive scalar
- Add documentation aliases to `f16b` - Add Wikipedia link to `bfloat16` - Remove `@ only-x86_64` flag from test
- ICE on `sparc64`, `mips64` & WASM
4ff6c4b to
fc04ded
Compare
This comment has been minimized.
This comment has been minimized.
fc04ded to
6754cda
Compare
This comment has been minimized.
This comment has been minimized.
6754cda to
b5496fb
Compare
This comment has been minimized.
This comment has been minimized.
b5496fb to
505a38a
Compare
This comment has been minimized.
This comment has been minimized.
505a38a to
8b18e08
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot)Important For more information how to resolve CI failures of this job, visit this link. |
|
re
by delaying adding support you now exclude a bunch of tests from the GCC run that we later need to enable again. I believe hooking it up is actually kind of trivial, so that may be the better call? |
| PartialEq, sym::eq, eq_trait, Target::Trait, GenericRequirement::Exact(1); | ||
| PartialOrd, sym::partial_ord, partial_ord_trait, Target::Trait, GenericRequirement::Exact(1); | ||
| CVoid, sym::c_void, c_void, Target::Enum, GenericRequirement::None; | ||
| BFloat, sym::bfloat, bfloat_type, Target::Struct, GenericRequirement::Exact(0); |
There was a problem hiding this comment.
| BFloat, sym::bfloat, bfloat_type, Target::Struct, GenericRequirement::Exact(0); | |
| F16B, sym::f16b, f16b, Target::Struct, GenericRequirement::Exact(0); |
May as well keep the lang item name consistent with the type name.
View all comments
Implements the RFC: f16b type. Best reviewed commit by commit, happy to split into separate PRs if that is deemed easier to review. However the line count and surface area is, in my opinion, reasonably small.
Adds;
f16balong withbfloatlang item to work with LLVM, GCC is explicitlyunimplemented!(...)f16bfeature gate, page forf16bon libruscdoc and astruct bf16incore::numf16bas a scalar primitive for scalable vectorsIssues;